CI: force software rendering to fix flaky VTK off-screen bus error (#1078) - #1084
Conversation
|
Could a maintainer approve the workflow run on this PR when you get a chance? Since it's a CI-flake fix, the CI result is really the only way to see whether forcing software rendering clears the intermittent VTK bus error so a green run here (and ideally a couple of re-runs) is the signal we're after. No library code is touched, only the two test-workflow env blocks. Thanks! |
Approved the run! I believe the error persists, do you have any guesses on why? |
Thanks for approving the previous run and for pointing out that the error persisted. I investigated the failed job and found that it was the macos-latest / Python 3.14 matrix leg. The integration-test process The original Mesa settings did not affect this failure because LIBGL_ALWAYS_SOFTWARE and GALLIUM_DRIVER=llvmpipe control Mesa’s Linux-style OpenGL stack. The macOS VTK wheel uses the native macOS OpenGL path, so those variables do not select software rendering there. I pushed follow-up commit 15968a8, which:
GitHub has created new Tests and Linters workflow runs for the commit, but both are currently waiting for maintainer approval
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1084 +/- ##
===========================================
+ Coverage 82.18% 82.36% +0.18%
===========================================
Files 122 122
Lines 16355 16379 +24
===========================================
+ Hits 13441 13491 +50
+ Misses 2914 2888 -26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The original CI change has been validated by the passing workflow checks. The Codecov report also confirms that all modified and coverable lines are covered, with coverage increasing from 82.18% to 82.36%. |
Thanks for the updates. Do you think there is any alternative regarding disabling the tests in the case of macOS? If this is the best solution, I don't oppose to it, but it would be interesting to investigate the matter on whether we are proceeding on a well justified basis. Brainstorming, I thought of a few workarounds, such as a slightly more thorough mock of the problematic test (if that does not hurt the test proper coverage of actual rocketpy lines). Furthermore, I see that the disabling rule is specifically for |
|
Thanks for the thoughtful feedback! I agree that the previous macOS/Python 3.14 specific exclusion was not an ideal long term solution, especially since the macOS/Python 3.10 job was passing. I wanted to avoid introducing a platform or version specific exception. I ended up taking a different approach:
My goal was to keep the tests themselves intact rather than mock or permanently disable them. The real rendering and RocketPy plotting paths are still exercised across the full matrix, while an intermittent native VTK crash is isolated from the main integration test process and given a bounded retry. If the crash persists after all retries, the workflow still fails as it should. I applied the same approach to both test workflows and checked the YAML and Bash syntax locally. I'd be interested to hear what you think. If you see a cleaner way to handle this, I'm happy to adjust the implementation. |
Gui-FernandesBR
left a comment
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
I approved the pending workflow runs, so a22b95d2 has now been exercised for the first time — the earlier green run (30561124560) was 4d988a53, which still had the if [[ macOS && 3.14 ]] branch, so the isolated-step version had never actually run. Result on a22b95d2 (run 30678915539): all six matrix legs green, and on macOS/3.14 the new Run VTK animation tests step ran and passed on its own.
Better still, we got an unplanned control experiment. I approved #1085's run at the same time, on the same afternoon and against the same develop. It crashed:
ubuntu-latest, 3.14 | Run Integration Tests
Fatal Python error: Segmentation fault
pytest tests/integration --cov=rocketpy --cov-append
##[error]Process completed with exit code 139
Same develop, no isolation, dead. Yours, with the animation tests pulled into their own step, green across the board. That is about as close to a paired comparison as a flake allows, and it argues for merging this.
One thing I would still change before merging, because it silently does nothing today:
if [[ "$status" != "138" || "$attempt" == "$attempts" ]]; then138 is 128+10, and signal 10 is SIGBUS on macOS/BSD only. The crashes actually recorded in this repo are:
| run | crash | exit code |
|---|---|---|
| develop 29668287288 | Bus error: 10 |
138 |
| develop 29697106388 | Bus error: 10 |
138 |
| #1098 31230909959 (macOS) | Segmentation fault: 11 |
139 |
| #1085 31077063979 (Linux) | Segmentation fault (core dumped) |
139 |
So the two most recent crashes — including the one from this very afternoon — are SIGSEGV/139 and would fall straight through the retry on the first attempt. On Linux the constant is wrong for bus errors too: there SIGBUS is 7, so a Linux bus error exits 135 and never retries either. I confirmed the control flow with a stub under the same flags Actions uses (bash -eo pipefail): 138 retries three times; 139 and 135 exit immediately.
Since the observed distribution is 8 SIGSEGV to 2 SIGBUS, the retry as written misses the common case. Accepting the whole native-crash family fixes it:
case "$status" in
135|138|139) ;; # SIGBUS (Linux/macOS), SIGSEGV
*) exit "$status" ;; # real test failure: fail fast
esac
[[ "$attempt" == "$attempts" ]] && exit "$status"Two notes, neither blocking:
LIBGL_ALWAYS_SOFTWARE/GALLIUM_DRIVERare correctly gated torunner.os == 'Linux', but every crash we had logged until today was on macOS, where those variables do nothing (Apple's GL, not Mesa). So the "software rendering" in the title is not what is carrying this PR — the isolated step is. Worth retitling to match, since the Mesa vars are still worth keeping for the Linux SIGSEGV we just saw.- In
test-pytest-slow.yamlthe vars sit in the job-levelenv:with no guard, which is fine there because that job is pinned toruns-on: ubuntu-latest.
I also verified the three --deselect node IDs all resolve to real tests (tests/integration/test_plots.py:12, :36, :73), so nothing is being silently skipped by a typo.
Last thing: fail-fast: false is a genuinely good addition and is exactly why #1085's run reported 1 failure and 5 cancelled. Heads up that @thc1006 has just opened #1100 doing only that part, so one of the two will need a trivial rebase depending on merge order — I would suggest landing this one first, since it is the superset.
Gui-FernandesBR
left a comment
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Approving on the strength of the run I just released: all six matrix legs green on a22b95d2, with the isolated Run VTK animation tests step passing on macOS/3.14 — and #1085's run, released at the same time against the same develop without this change, dying on SIGSEGV in the un-isolated integration step.
Merging as-is because this unblocks the CI for every other open PR, and the retry's exit-code narrowness is a missed opportunity rather than a regression: without it we simply keep the current behaviour for SIGSEGV. The case "$status" in 135|138|139) fix from my previous comment is still worth doing, and #1100 will need a trivial rebase since fail-fast: false lands here first — happy to take either as a follow-up.
a22b95d to
ac085b9
Compare
RocketPy-Team#1084 added fail-fast: false to the main test matrix while this was open, so the only half left is the slow one. Same reason: 3.10 failing says nothing about 3.14, so cancelling it costs a result and saves nothing worth having. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
RocketPy-Team#1084 retries the animation tests when they die on 138, which is SIGBUS on macOS. Counting the last 40 Tests runs, the crash was 139 eight times and 138 twice, so the common case fell straight through the retry. Linux SIGBUS is 135 rather than 138, so that missed as well. Also sets fail-fast: false on the slow matrix, which RocketPy-Team#1084 left out. 3.10 failing says nothing about 3.14, so cancelling it costs a result and saves nothing worth having. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
#1100) * MNT: do not let one Python version cancel the other in the slow matrix #1084 added fail-fast: false to the main test matrix while this was open, so the only half left is the slow one. Same reason: 3.10 failing says nothing about 3.14, so cancelling it costs a result and saves nothing worth having. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> * MNT: retry the VTK tests on SIGSEGV, not only on macOS SIGBUS #1084 retries the animation tests when they die on 138, which is SIGBUS on macOS. Counting the last 40 Tests runs, the crash was 139 eight times and 138 twice, so the common case fell straight through the retry. Linux SIGBUS is 135 rather than 138, so that missed as well. Also sets fail-fast: false on the slow matrix, which #1084 left out. 3.10 failing says nothing about 3.14, so cancelling it costs a result and saves nothing worth having. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --------- Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
3 participants
Pull request type
Checklist
yaml.safe_load)CHANGELOG.md— n/a per the changelog's own "should not be here: github maintenance"Current behavior
tests/integration/test_plots.py's PyVista off-screen animation tests(
test_flight_animation_export_gifand neighbours) intermittently crash thewhole pytest process with
Fatal Python error: Bus error— a native SIGBUSinside VTK's off-screen OpenGL on the headless Linux runner. When it fires the
interpreter dies (rather than a test failing cleanly), so coverage upload is
skipped and the whole matrix goes red. It also hits
developdirectly. This is #1078.New behavior
Forces Mesa software rendering on the test jobs by setting
LIBGL_ALWAYS_SOFTWARE=1andGALLIUM_DRIVER=llvmpipe. Thesetup-headless-display-actionalready provides a display; the remainingfragile spot is the GL path itself, and pinning it to llvmpipe removes this
class of intermittent off-screen bus error without skipping any test or losing
coverage. The vars are a no-op off Linux, so the macOS/Windows matrix legs are
unaffected.
Why not the alternatives (from the issue)
pytest-rerunfailuresalone can't help — a SIGBUS kills the interpreter,so there's nothing left to rerun.
pytest-forkedwould isolate the crash but needsos.fork, and thematrix includes
windows-latest.Software rendering targets the root cause instead. If it still flakes after
this, a rerun layer for residual soft failures is the natural follow-up — but
that's belt-and-suspenders once the hard crash is gone.
Breaking change
Additional information
Being a CI flake, this can't be proven fixed from a single run — but forcing
llvmpipe is the standard, low-risk mitigation for VTK off-screen bus errors on
GitHub headless runners, and it changes nothing about the library or the tests.
Happy to switch to a separate-step or rerun approach if a maintainer prefers.
Closes #1078